feat(sdk-core): getMpcV2RetrofitDataFromMpcV1Keys for EdDSA - #9461

WCI-1263

@claude

Claude finished @Marzooqa's task —— View job

Code Review Complete ✅

Reviewing the EdDSA MPCv2 retrofit implementation for WCI-1263.

Todo List:

• Review EddsaRetrofitData type definition
• Analyze getMpcV2RetrofitDataFromMpcV1Keys implementation
• Review private helper getMpcV2RetrofitDataFromMpcV1Key
• Check unit tests coverage and quality
• Verify RFC 8032 compliance and security practices
• Provide comprehensive feedback

Review Summary

This PR successfully implements EdDSA MPCv2 retrofit functionality for migrating from MPCv1 to MPCv2 keys. The implementation is solid and addresses the requirements well.

Key Strengths

1. Correct Architectural Approach

• Properly uses MPC.keyCombine() instead of manual scalar derivation (fixed from earlier version)
• Correctly derives retrofit data from pShare.u/pShare.y/pShare.chaincode output
• Validates that user and backup keys produce consistent aggregate public key and chain code

2. Type Safety & Location

• EddsaRetrofitData type is well-defined with clear documentation
• Moved to @bitgo/sdk-lib-mpc/src/tss/eddsa-mps/types.ts for better organization (mirrors ECDSA pattern)
• Type structure matches expected MPS DKG ceremony requirements

3. Robust Input Validation

• Comprehensive assertions for all required MPCv1 key material fields (uShare, bitgoYShare, party-specific YShares)
• Clear error messages for missing fields distinguish between user and backup context
• JSON.parse error handling for malformed input

4. Excellent Test Coverage

• 11 unit tests covering all critical paths and edge cases
• Tests use real MPCv1 key shares generated via MPC.keyShare() rather than fixtures
• Edge case coverage: missing fields, empty strings, wrong party configurations
• Validation tests confirm user/backup keys produce matching expectedPk and chainCode

5. Security Considerations

• No longer manipulates raw seed bytes or performs manual SHA-512/clamping
• Delegates cryptographic operations to the existing, battle-tested MPC.keyCombine() logic
• Maintains the same security model as existing MPCv1 EdDSA operations

Implementation Details

Public Interface (lines 1073-1099)

async getMpcV2RetrofitDataFromMpcV1Keys(params: { 
  mpcv1UserKeyShare: string; 
  mpcv1BackupKeyShare: string 
}): Promise<{
  userRetrofitData: MPSTypes.EddsaRetrofitData;
  backupRetrofitData: MPSTypes.EddsaRetrofitData;
}>

• Clean async API accepting raw MPCv1 key share strings
• Returns structured retrofit data for both parties
• Cross-validates that both parties agree on aggregate key material

Private Helper (lines 1101-1135)

private static getMpcV2RetrofitDataFromMpcV1Key(
  mpcv1PartyKeyShare: string,
  mpcv1PartyIndex: MPCv2PartiesEnum.USER | MPCv2PartiesEnum.BACKUP,
  mpc: Awaited<ReturnType<typeof getInitializedMpcInstance>>
): MPSTypes.EddsaRetrofitData

• Proper separation of concerns - handles single party processing
• Uses MPCv2PartiesEnum for type-safe party identification
• Mirrors pattern from existing ECDSA implementation

Minor Suggestions

1. Error Message Consistency: Consider standardizing error message format across all validation checks
2. Performance: The MPC instance initialization could potentially be cached if this becomes a hot path
3. Documentation: Consider adding JSDoc examples for the expected MPCv1 key share JSON structure

Test Quality Assessment ⭐⭐⭐⭐⭐

The test suite is exceptionally thorough:

• Positive cases: Real key generation and successful retrofit
• Input validation: Missing fields, malformed JSON, type mismatches
• Cross-validation: Ensures user/backup consistency
• Integration: Uses real MPC library calls instead of mocks where appropriate

Security Assessment ✅

• No sensitive data exposure: Operates on existing MPCv1 key material without exposing additional secrets
• Cryptographic correctness: Uses the same keyCombine logic as production EdDSA flows
• Input sanitization: Proper validation before passing data to cryptographic functions

Conclusion

This is a well-implemented feature that successfully enables MPCv1 to MPCv2 migration for EdDSA keys. The code is secure, well-tested, and follows BitGoJS architectural patterns. The fix to use real keyCombine() instead of manual derivation significantly improves correctness and maintainability.

Recommendation: ✅ Approve

Job: PR Comment | Branch: WCI-1263-getMpcV2RetrofitDataFromMpcV1Key-EdDSA